Code Review: fix(mocks): implement indexer in generated mock (#5676)

Overall, this is a well-structured and complete fix. The approach of dispatching to dedicated GenerateInterfaceIndexer / GenerateOverrideIndexer methods — rather than simply un-skipping the existing property path — is the right call; indexers have different codegen shape (no named member, this[…] syntax, implicit value in setters) and would have needed if-guards throughout the property generator. The fallback semantics in GenerateOverrideIndexer cleanly mirror the existing override-property pattern.

Things done well

• Fallback correctness. The try-then-fallback pattern in GenerateOverrideIndexer for non-abstract members (TryHandleCallWithReturn / TryHandleCall, then delegate to base[…] or _wrappedInstance[…]) correctly mirrors the same pattern used in the existing override property helpers.

• Extension method naming. Choosing Item(…) / SetItem(…, value) maps directly to the get_Item / set_Item IL names used in the engine call strings. Consistent and unsurprising.

• Edge-case allocation. Using Array.Empty<object?>() in the zero-parameter getter path (and for the matcher list) avoids a pointless allocation. The setter path always allocates (because value must be in there), which is correct.

• Test coverage. The two T14 tests cover: compile with a co-resident regular property, distinct-index independence, exact call-count verification, mixed literal / wildcard matchers, and setter verification. That's a solid regression baseline.

Issues and suggestions

1. XML doc comments on private methods — against project coding standards

GenerateInterfaceIndexer, GeneratePartialIndexer, GenerateWrapIndexer, and GenerateOverrideIndexer all carry multi-line <summary> blocks. CLAUDE.md is explicit: "Default to writing no comments. Only add one when the WHY is non-obvious." Private generator helpers with self-documenting names don't meet that bar; the /// <summary> blocks describe what the code does (already clear from the name) rather than why a non-obvious constraint exists. Removing them keeps the file consistent with the rest of the builder.

2. FormatIndexerParameterList silently drops parameter modifiers

private static string FormatIndexerParameterList(MockMemberModel indexer)
{
    return string.Join(", ", indexer.Parameters.Select(p => $"{p.FullyQualifiedType} {p.Name}"));
}

This only emits type name; it doesn't emit in, ref, or out modifiers. Indexers with in T key parameters (a valid C# pattern) would generate code that doesn't compile. Consider checking whether MockMemberModel.ParameterModel carries modifier information and forwarding it here, similarly to how the existing method-parameter formatting handles it. If parameter-with-modifiers indexers are genuinely impossible in the current model, a guard or comment is worthwhile so future model changes don't silently break this.

3. argsArray is computed unconditionally in two places

In GenerateInterfaceIndexer:

var argsArray = GetIndexerGetterArgsArray(prop);   // always runs
// ...
if (prop.HasGetter)
{
    writer.AppendLine($"get => ... {argsArray} ...");
}

And identically in GenerateOverrideIndexer. If the indexer is setter-only, argsArray is built but never used. This is a very minor point (it's pure string work), but it's also trivially fixed by moving the variable inside the if (prop.HasGetter) block, which improves reading clarity (the reader sees the value and its use together).

4. Known limitation — typed wrapper not emitted for indexer interfaces

The PR description acknowledges MockWrapperTypeBuilder.CanGenerateWrapper still returns false for indexer-bearing interfaces. Worth tracking as a follow-up issue if it isn't already filed, because the IFoo_Mock typed wrapper is the primary ergonomic surface for most consumers; landing without it means indexer-bearing interface users get Mock<IFoo> which is functional but less convenient.

Snapshot tests

CLAUDE.md requires that changes to source generator output have their .verified.txt snapshots committed. The diff doesn't include snapshot files — assuming they were reviewed and are committed, this is fine. If they haven't been updated, running the snapshot-acceptance loop described in CLAUDE.md is needed before merge.

Verdict: The core implementation is correct and well-designed. The two issues worth addressing before merge are the XML doc comments (minor, style) and the missing parameter-modifier forwarding in FormatIndexerParameterList (correctness gap for a currently-rare but valid input). The unconditional argsArray is cosmetic only.

Code Review: Implement Indexer in Generated Mock

Summary

This PR addresses #5676 by implementing full indexer support in the TUnit mocks code generator. Previously, interfaces with indexers were unmockable — the generator skipped them with if (prop.IsIndexer) continue;, producing CS0535 compilation errors. This PR:

1. Implements indexer emission across three code paths (interface, partial, wrapper mocks)
2. Adds Item(...)/SetItem(..., value) extension methods in MockMembersBuilder for fluent setup/verification
3. Restores previously-skipped tests (T14, T14b) in KitchenSinkEdgeCasesTests.cs
4. Fixes parameter direction handling for indexer parameters (in modifiers)

Architectural Assessment: Well-Designed

Three-path dispatch strategy is correct. The PR correctly separates:

• Interface mocks — direct engine routing (no fallback needed since all members are abstract)
• Partial mocks — try-intercept with base fallback
• Wrapper mocks — try-intercept with _wrappedInstance fallback

This mirrors exactly how methods and properties are handled, maintaining consistency. The try-intercept pattern allows non-mocked indices to fall through to real implementations, which is the correct semantic.

Extension method API is idiomatic. Exposing mock.Item(5).Returns("five") and mock.SetItem(5, "five").WasCalled() reuses MockMethodCall<T> and VoidMockMethodCall without modification — a strong sign the abstraction is being applied correctly rather than bent.

MemberDiscovery.cs fix is important. Replacing the hardcoded Direction = ParameterDirection.In with p.GetParameterDirection() was a correctness bug, not just a style issue. Without this, in-parameter indexers would be silently mishandled in generated code.

Issues Found

1. Multi-parameter indexer coverage gap (LOW risk, but worth noting)

There are no tests for indexers with multiple parameters (e.g., this[int x, int y]). The logic in GetIndexerGetterArgsArray() and GetIndexerSetterArgsArray() appears correct for this case — it iterates all parameters — but a test would confirm the generated Item(x, y) extension method compiles and dispatches correctly. Worth a follow-up issue if not in scope now.

2. Wrapper type generation remains disabled (acknowledged limitation)

MockWrapperTypeBuilder.CanGenerateWrapper still returns false for indexer-bearing interfaces. This is correctly acknowledged as out of scope. A follow-up issue tracking this would prevent it from being forgotten.

3. Static-abstract indexers remain unhandled (acknowledged limitation)

MockBridgeBuilder still skips static-abstract indexers. Also correctly scoped out.

What's Done Well

• The parameter direction fix in MemberDiscovery.cs is a genuine correctness improvement beyond just the feature work
• Tests T14a/T14b/T14c cover the golden path, distinct-index isolation, wildcard matchers, and in parameters
• Generated code follows the same HandleCallWithReturn/HandleCall dispatch pattern as existing methods — no new engine surface area needed
• All 954+ existing tests continue to pass

Verdict

Approve. The implementation is sound, consistent with existing patterns, and well-tested for the primary use cases. The known limitations are properly scoped and acknowledged. The parameter direction fix is a bonus correctness improvement.

The only actionable suggestion is to consider opening follow-up issues for: (1) multi-parameter indexer tests, (2) wrapper type generation for indexer interfaces, so these don't get lost.